lakebox: clean comments and drop bugbash dir for open-source publication#5459
Merged
akshaysingla-db merged 3 commits intoJun 6, 2026
Merged
Conversation
Scrubs internal-only references from cmd/lakebox/ in preparation for this code shipping in a public CLI release: - Drops employee-name attributions in code comments (config.go, list.go, start.go, state.go). - Drops bug-bash form IDs (F10, F22) — the technical context they hung off stands on its own. - Drops references to private-repo paths (lakebox/src/..., universe#1966484, lakebox.proto) and to internal SAFE flag names (lakeboxCheckerEnabled), internal subsystems (ESM, LakeboxChecker), and internal vocabulary (dogfood staging). - Drops the "no prod listener yet" roadmap mention from resolveGatewayHost — advertising rollout state isn't useful to OSS readers. - Fixes lakebox status help-text examples to include the `databricks` prefix for consistency with sibling commands. - Removes internal/bugbash/ entirely (install.sh + exec.sh + README). The snapshot-install script was an internal coordination tool tied to the demo-lakebox branch lifecycle and doesn't belong in a public repo. The release-build workflow still triggers on `bugbash-*` branches as a benign CI convention. No behavior change; pure comment + dead-file cleanup. Tests still pass. Co-authored-by: Isaac
Contributor
Approval status: pending
|
The previous commit deleted the entire `internal/bugbash/` directory, but exec.sh and README.md predate the lakebox work — Pieter authored them in databricks#1844 (May 2024), Lennart and Andrew extended them, and they are linked via a public curl-pipe-bash URL that may be referenced from internal Slack/docs. Removing them without consulting the maintainers is out of scope for an OSS-comment-cleanup PR. Only install.sh stays deleted: it was added in databricks#5359 specifically to install the demo-lakebox snapshot, and the long-term home for that script is no longer inside the public repo. Co-authored-by: Isaac
a1f1625 to
a7aa755
Compare
Pass over every Go file in cmd/lakebox/ to bring comment density in
line with the repo style guide (comments explain WHY, not WHAT; default
to none). Net 152 lines removed.
What got cut:
- Pure noise that restated obvious code ("Use the dedicated lakebox
SSH key.", "Check that ssh-keygen is available before trying to
generate.", "Slice off the OpenSSH comment by stopping at the second
space.", etc.).
- Multi-paragraph essays where one sentence was load-bearing.
- Duplicated explanations across function-doc and call-site, or
across sibling functions (stop/start/registerKey, sandboxEntry/
updateBody).
- Code-history narration ("used to render a hardcoded 10m...", "until
this helper existed lakebox commands silently...").
- Signature restatements at the end of function docs.
What was preserved (the exemplars worth keeping):
- sandboxPath security context.
- idleTimeoutSecs Duration proto3 form.
- listPage SDK slot-6 quirk.
- verifyKeyRegistered SSH protocol limitation.
- buildSSHArgs ssh-arg-joining trap.
- removeSandbox orphan-gateway-host cleanup.
- buildSSHConfigBlock UI alignment + first-match-wins ordering.
- field SGR-before-padding invariant.
Added missing function-doc comments to New, loadState, saveState,
stateFilePath for consistency with the rest of the package.
No behavior change.
Co-authored-by: Isaac
2e5b9e5
into
databricks:demo-lakebox
12 of 19 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pre-flight cleanup for shipping
cmd/lakebox/to the public CLI release. No behavior change — pure comments + dead-file removal. After this, the lakebox subfolder is appropriate for the publicdatabricks/clirepo.What got changed
config.go,list.go,start.go,state.gossh.go,ssh_test.go,start.goF10,F22references were internal coordination tags meaningless to external readers.api.go,config.go,ssh.golakebox/src/...,universe#1966484,lakebox.proto,LakeboxService.*proto refs — external readers can't navigate to any of them.api.goESM,LakeboxChecker,lakeboxCheckerEnabledSAFE flag, "dogfood staging"ssh.gostatus.godatabricks lakebox status …to match sibling commandsinternal/bugbash/install.shWhat was deliberately kept
internal/bugbash/exec.shandinternal/bugbash/README.md— these predate lakebox (authored by Pieter in Add script to make testing of code on branches easier #1844, May 2024; extended by Lennart and Andrew). They expose a curl-pipe-bash URL that may be referenced elsewhere; removing them is out of scope for a lakebox-comment cleanup. Whoever revisits the public surface ofinternal/bugbash/can decide separately.verifyKeyRegistered—USERAUTH_FAILUREreason-poverty /USERAUTH_BANNERswallowing is genuinely educational and applies to any SSH gateway, not just lakebox.validate_sandbox_id/foo;rm -rf /security context — load-bearing.databricks-sdk-gomakeRequestBodyquirk reference —databricks-sdk-gois a public repo.cmdio.Width/cmdio.PadRightwidth-aware padding rationale.Notes for follow-up
release-build.ymltriggers onbugbash-*branches. This is a CI convention pattern (matches any branch starting withbugbash-), not a link to the deleted install.sh. Benign enough to keep — one-line follow-up if we want it scrubbed.Test plan
go test ./cmd/lakebox/...passesgo build ./...cleangofmt -l cmd/lakebox/cleancmd/lakebox/(verified by grep sweep for names, F-IDs, internal paths)This pull request and its description were written by Isaac.